Skip to content

HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan.#10189

Open
slfan1989 wants to merge 2 commits into
apache:masterfrom
slfan1989:HDDS-15174
Open

HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan.#10189
slfan1989 wants to merge 2 commits into
apache:masterfrom
slfan1989:HDDS-15174

Conversation

@slfan1989
Copy link
Copy Markdown
Contributor

@slfan1989 slfan1989 commented May 5, 2026 •
edited
Loading

What changes were proposed in this pull request?

This PR adds unit test coverage for the Ozone Iceberg statistics file copy plan helper.

RewriteTablePathOzoneUtils.statsFileCopyPlan is used by the Ozone Iceberg RewriteTablePath implementation to build copy plan entries for table statistics files. The helper validates that the original and rewritten statistics file lists match in count and file size, and then creates mappings from the original statistics file paths to the rewritten statistics file paths.

This PR adds tests for the following cases:

  • empty statistics file lists
  • mismatched before/after statistics file counts
  • mismatched statistics file sizes
  • successful before-to-after path pair generation

The change improves coverage for the Iceberg rewrite path utility without changing production behavior.

What is the link to the Apache JIRA

HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan.

How was this patch tested?

  • Tested with
mvn -pl hadoop-ozone/iceberg test
  • Result
[INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.674 s -- in org.apache.hadoop.ozone.iceberg.TestRewriteTablePathOzoneAction
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

sreejasahithi
sreejasahithi reviewed May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @slfan1989 for adding tests for statistics file copy plan. left a minor comment below.

Comment thread ...e/iceberg/src/test/java/org/apache/hadoop/ozone/iceberg/TestRewriteTablePathOzoneAction.java
Comment on lines +202 to +211
@Test
void statsFileCopyPlanRejectsMismatchedStatsCount() {
IllegalArgumentException exception = assertThrows(IllegalArgumentException.class,
() -> RewriteTablePathOzoneUtils.statsFileCopyPlan(
List.of(statisticsFile("before-1.puffin", 100)),
List.of()));

assertEquals("Before and after path rewrite, statistic files count should be same",
exception.getMessage());
}
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi May 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statistics files always have the .stats extension in real usage and .puffin is the extension used for Deletion Vector files (*-deletes.puffin). While the test works with any string since statsFileCopyPlan never validates the extension, using .puffin is misleading. Should be .stats

Copy link
Copy Markdown
Contributor Author

@slfan1989 slfan1989 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! Fixed. The test data now uses .stats for statistics files, which better matches the real file naming and avoids implying deletion vector files.

sreejasahithi
sreejasahithi approved these changes May 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @slfan1989
LGTM

@sreejasahithi
Copy link
Copy Markdown
Contributor

sreejasahithi commented May 11, 2026

@slfan1989 could you please resolve the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sreejasahithi sreejasahithi sreejasahithi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@slfan1989 @sreejasahithi @adoroszlai